Skip to content

fix(auth): declare 'loginparams' capability so the server selector works regardless of auth driver#91

Open
jcdelepine wants to merge 2 commits into
horde:FRAMEWORK_6_0from
jcdelepine:fix/imp-server-selector-login
Open

fix(auth): declare 'loginparams' capability so the server selector works regardless of auth driver#91
jcdelepine wants to merge 2 commits into
horde:FRAMEWORK_6_0from
jcdelepine:fix/imp-server-selector-login

Conversation

@jcdelepine

Copy link
Copy Markdown
Contributor

IMP_Application::authLoginParams() (which builds the server selector when $conf['server']['server_list'] = 'shown') was only reachable via Horde_Core_Auth_Application::getLoginParams() when the Horde auth driver was 'application' pointed at imp. Declaring 'loginparams' in $auth lets horde/base's LoginService/login.php resolve these params independently, via hasCapability('loginparams').

works regardless of auth driver

IMP_Application::authLoginParams() (which builds the server selector
when $conf['server']['server_list'] = 'shown') was only reachable via
Horde_Core_Auth_Application::getLoginParams() when the Horde auth
driver was 'application' pointed at imp. Declaring 'loginparams' in
$auth lets horde/base's LoginService/login.php resolve these params
independently, via hasCapability('loginparams').
@TDannhauer

Copy link
Copy Markdown
Contributor

Thanks for the PR! I looked at this together with the companion PR horde/base#136 (this change is only effective in combination with it — nothing in the currently shipped code checks hasCapability('loginparams')).

Mechanically the capability declaration works: Horde_Core_Auth_Application::hasCapability() reads IMP_Application::$auth, and the _bootstrap() logic that strips add/list/remove when no admin config is present doesn't affect it.

However, I think the pair of PRs doesn't fully fix the underlying problem — see my detailed comment on horde/base#136. Short version: with a non-application Horde auth driver, the posted imp_server_key is never consumed. It is only read in IMP_Application::authAuthenticate(), which only runs when IMP's own auth driver authenticates — exactly the driver = 'application' → imp configuration that already worked. Neither the POST-collection paths (login.php, LoginService::processLogin()) nor IMP's transparent auth (IMP_Auth::transparent()_canAutoLogin()getAutoLoginServer()) look at the selection, so the dropdown would render but the chosen server would be silently ignored.

The root cause is more likely a small omission in LoginService::buildLoginFormData() (details in the base PR comment). If that is fixed, the existing generic app-auth flow renders the selector and honors the selection, without a new capability or an imp special-case in horde/base.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants